-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drpcmanager: cancel stream with error when terminated #31
Conversation
Terminations were canceling streams with `context.Canceled` instead of surfacing the error, causing unexpected behavior.
@zeebo could you take a look? |
Hmm.. I think this changes some semantics. Specifically, a |
Is there another way we should surface this error? Because this error wasn't being surfaced, it was very difficult for us to log the true reason a stream was being terminated. |
Perhaps the manager code can check to see if the error was |
Or, maybe we do something like |
The storj/drpc#31 PR was not merged, but was replaced by: storj/drpc@9206537 This should fix the underlying issue fixed in the fork.
The storj/drpc#31 PR was not merged, but was replaced by: storj/drpc@9206537 This fixes the underlying issue fixed in the fork.
This pull request has been mentioned on Storj Community Forum (official). There might be relevant details there: https://forum.storj.io/t/introducing-drpc-our-replacement-for-grpc/13486/13 |
Terminations were canceling streams with
context.Canceled
insteadof surfacing the error, causing unexpected behavior.